[Repo Assist] fix: prevent duplicate test ID crash for xUnit MemberData; improve theory name parsing; add TypeCheck build target#2156
Draft
github-actions[bot] wants to merge 2 commits intomainfrom
Conversation
…TypeCheck build target - In ArrayExt.venn, add Array.distinctBy guards before building the ID maps to prevent 'An item with the same key has already been added' exceptions when xUnit MemberData theory tests produce duplicate IDs (Closes #2126, works alongside PR #2127) - In TestItemDTO.getFullname_withNestedParamTests for XUnit, replace the Split('.') |> Array.last approach with Substring(FullName.Length). This correctly handles float parameters like 0.5 in InlineData and parameter records in MemberData that contain dots in their ToString output. - Add a TypeCheck FAKE target to build/Program.fs that runs Fable compilation only (no webpack bundling), providing faster type-checking feedback in development and CI scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
39 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary
Closes #2126 (the "An item with the same key has already been added" crash for xUnit
MemberDatatests — works alongside draft PR #2127 which addressesInlineDatafloat parameters).Changes
1.
ArrayExt.venn— defensive deduplication guardvennbuildsDictionarymaps from test IDs. If twoTestItemDTOentries resolve to the same ID (as happens withMemberDatatheory tests when xUnit generates no unique display name), thedict |> Dictionaryconstruction throws:Fix: add
Array.distinctBybefore building each map. This silently drops duplicates rather than crashing, so the remaining test items are still shown and runnable.2.
TestItemDTO.getFullname_withNestedParamTests— safer XUnit theory fragment extractionThe old approach used
dto.DisplayName.Split('.') |> Array.last. For display names like"MyModule.Returns expected value ({ Multiplicand = 0.5; ExpectedValue = 1.0 })"this yields"0 })"— wrong, and a duplicate key if multiple cases share the same tail fragment.Fix: when
DisplayNamestarts withFullName, useSubstring(FullName.Length)to extract the parameter fragment without splitting on interior dots. This matches the approach in draft PR #2127 (by GitHub Copilot) and extends it to the MemberData case where the xUnit display name includes field values with decimal points.3.
build/Program.fs— newTypeCheckFAKE targetAdds a
TypeChecktarget that runs Fable compilation only (no webpack bundling). This is faster than the fullBuildtarget and is useful as a quick type-correctness check in development or lightweight CI scenarios.Root Cause
xUnit's VSTest adapter does not include theory case parameters in
FullyQualifiedName(FullName). For[(InlineData)]tests, xUnit appends the parameter values toDisplayName; for[(MemberData)]tests the parameter objects are serialised via theirToString(). WhenToString()values contain dots (e.g. F# records withfloatfields), the oldSplit('.') |> Array.lastheuristic produces wrong/duplicate fragments. Thevenncrash is a secondary consequence of identical IDs entering the test tree.Test Status
✅ Fable compilation (
dotnet fable src --noCache) — no new errors or warnings.✅ Build script compilation (
dotnet build build/build.fsproj) — no errors.No automated test suite exists. The fix is manually verifiable by creating an xUnit project with
[(MemberData)]theory tests using F# records as test data and confirming the Test Explorer no longer crashes on refresh.